Let `ChannelSigner` set and spend LN scriptpubkeys #3512

tankyleo · 2025-01-08T19:47:45Z

This PR allows the customization of different outputs of the commitment transaction, in preparation for taproot channels and also to allow people to set the outputs to arbitrary scripts if they don't require compatibility with the formal LN spec.

My approach is to ask the channel signers to do more work (but not more than that of most hardware wallets):

Set the appropriate SPK's for the different outputs of the commitment transaction.
Let the various transaction builders know of the expected weight of the witness to spend such an output - the transaction builders can then set appropriate feerates when building the transactions.
At spend time, return a finalized witness for the specified input - previously the EcdsaChannelSigner would return just the signature.

This PR:

Assumes that if the commit tx htlc output is customized to something outside the LN spec, that channel supports zero fee htlc tx.
Assumes that if an output of the commit tx is customized, the output remains the same type (P2WPKH, P2WSH)
The helper functions SpendableOutputDescriptor::to_psbt_input and SpendableOutputDescriptor::create_spendable_outputs_psbt assume scripts from the LN specification are used.

I mark it as draft. I understand this is a huge PR. I hope to show the full picture of how the current approach works to get approach ACKs, then happy to break it down into smaller PRs.

Remaining TODOs:

Update SpendableOutputDescriptor to accomodate taproot / arbitrary scripts.
Consider asking the ChannelSigner to return just the witness that finalizes the given input, instead of returning the full transaction with the given input finalized.
Figure out a mechanism to guide people who want to implement a ChannelSigner inline with the LN spec. One possible route: have the ChannelSigner trait offer "ProvidedMethods" for implementers who want to stay in-line with the LN specification - people who want to depart from the spec would then override these methods.
Work out how the validation in FundedChannel will work given a customized commitment transaction.
Customize the funding output, partially complete (blocked on custom p2p messages, and custom commit tx weight).
Custom commitment transaction weight (blocked on TRUC transactions).
[taproot]: Expand witness calls to include all the bitcoin::sighash::Prevouts of the transaction that the input is spending.
Documentation, and more detailed commit msgs.
Finalize EcdsaChannelSigner vs ChannelSigner APIs.

Supersedes #3454

codecov · 2025-01-13T18:39:29Z

Codecov Report

Attention: Patch coverage is 93.55783% with 44 lines in your changes missing coverage. Please review.

Project coverage is 88.81%. Comparing base (ad462bd) to head (8a20905).
Report is 11 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/util/test_channel_signer.rs	54.16%	2 Missing and 31 partials ⚠️
lightning/src/ln/chan_utils.rs	95.06%	2 Missing and 2 partials ⚠️
lightning/src/sign/mod.rs	99.08%	2 Missing and 1 partial ⚠️
lightning/src/chain/channelmonitor.rs	96.00%	1 Missing and 1 partial ⚠️
lightning/src/chain/package.rs	98.43%	1 Missing ⚠️
lightning/src/events/bump_transaction.rs	95.65%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3512      +/-   ##
==========================================
+ Coverage   88.42%   88.81%   +0.39%     
==========================================
  Files         149      149              
  Lines      112959   115657    +2698     
  Branches   112959   115657    +2698     
==========================================
+ Hits        99883   102723    +2840     
+ Misses      10605    10497     -108     
+ Partials     2471     2437      -34

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

This allows the `to_remote` output to easily be changed according to the features of the channel, or the evolution of the LN specification. `to_remote` could even be set to completely arbitrary scripts if compatibility with the formal LN spec is not required.

This allows the `to_local` output to easily be changed according to the features of the channel, or the evolution of the LN specification. `to_local` could even be set to completely arbitrary scripts if compatibility with the formal LN spec is not required.

This allows the htlc tx output to easily be changed according to the features of the channel, or the evolution of the LN specification. The output could even be set to completely arbitrary scripts if compatibility with the formal LN spec is not required. Builders of htlc transactions now ask a `ChannelSigner` for the appropriate revokeable script pubkey to use, and then pass it to the htlc transaction constructors.

All LN-Penalty channel signers need to be able to punish the counterparty in case they broadcast an old state. In this commit, we ask implementers of `ChannelSigner` to produce the full transaction with the given input finalized to punish the corresponding previous output. Consumers of the `ChannelSigner` trait can now be agnostic to the specific scripts used in revokeable outputs. We leave passing to the `ChannelSigner` all the previous `TxOut`'s needed to produce valid schnorr signatures under BIP 341 spending rules to a later patch.

for the revokeable scripts in the `to_local` and the htlc tx outputs.

All LN-Penalty channel signers need to be able to punish the counterparty in case they broadcast an old state. In this commit, we ask implementers of `ChannelSigner` to produce the full transaction with the given input finalized to punish the corresponding previous output. Consumers of the `ChannelSigner` trait can now be agnostic to the specific scripts used in HTLC outputs of commitment transactions. We leave passing to the `ChannelSigner` all the previous `TxOut`'s needed to produce valid schnorr signatures under BIP 341 spending rules to a later patch.

…utputs`

…ner`

…t_signature`

TheBlueMatt · 2025-01-20T20:28:55Z

lightning/src/sign/mod.rs

+	///
+	/// If `is_holder_tx` is set, return the `to_remote` script pubkey for the local party's commitment
+	/// transaction, otherwise, for the remote party's.
+	fn get_counterparty_payment_script(&self, is_holder_tx: bool) -> ScriptBuf {


Just returning the scripts for each position in the commitment transaction is cool, but I don't think it would allow someone to have, for example, a dual-normal-DLC channel. Rather, it would be nice if all of build_commitment_transaction were the interface, tell the "signer" what the HTLCs are and let them build the whole transaction.

It may require a bit more work as we'll need to include additional metadata in the transaction object returned (which output is to_remote/to_local/each HTLC), but the ultimate result would be fewer API calls across the trait, a simpler API and more flexibility.

thanks for the review Matt - overall I agree, I'm researching this direction now.

@TheBlueMatt regarding this additional metadata:

right now we already have additional metadata returned, the Vec<(HTLCOutputInCommitment, Option<Box<HTLCSource>>)> for each HTLC included in the commit tx built with build_commitment_transaction.

this additional metadata travels to the corresponding ChannelMonitor via ChannelMonitorUpdates and gets stored in counterparty_claimable_outpoints: HashMap<Txid, Vec<(HTLCOutputInCommitment, Option<Box<HTLCSource>>)>>

When a counterparty commit tx falls onchain, we read that hashmap, and include that metadata in the PackageTemplates built, and these PackageTemplates are given to the OnchainTxHandler.

On the other hand for the to_local output of commitment transaction

In this PR, the ChannelMonitor does a roundtrip to the signer to ask it for the expected to_local script pubkey in that commitment transaction, then iterates over the outputs of the broadcasted transaction to identify the to_local output. The output is then passed to either a RevokedOutput PackageTemplate or DelayedPaymentOutputDescriptor depending on the case.

Question:

do we keep a single build_commitment_transaction call in channel, and then pass the data over to the ChannelMonitor via the updates, like is done now for HTLC outputs? In that case it seems we'd need to extend the updates and the state of the ChannelMonitor to include to_local outputs.

or do we call build_commitment_transaction again in ChannelMonitor to get the data needed to identify the to_local output.

I have a rough draft that implements the first path - send to_local output metadata via channel updates.

I am not thrilled about storing all the counterparty to_local outputs in the ChannelMonitor state. EDIT: we could store a mapping of the txid to the index of the to_local output in that commitment transaction.

But asking the signer to build the entire commitment transaction again seems repetitive (ie didn't I already give it to you?). Naively, we'd grab the HTLC metadata on the call in channel, and the to_local metadata in the second call in ChannelMonitor. We'd have to find a way to consolidate metadata collection for the ChannelMonitor in a single spot.

but the ultimate result would be fewer API calls across the trait, a simpler API and more flexibility.

I was hoping that with the Provided Methods we don't actually increase the number of methods most users would have to implement - just get_channel_parameters and get_channel_value_satoshis. Then people can override only the methods they require.

If we require additional outputs, with this current approach I was thinking of introducing one more call that returns a collection of these additional TxOuts. It would be a Provided Method that returns None / empty collection by default.

I still have to figure out how to offer the ability to set the locktime, and sequence fields of various transactions - definitely asking "signers" to build entire transactions would help in that regard.

Gah, somehow I totally missed this thread. Please feel free to ping me if I'm taking more than a day or two to respond. I assume this has been addressed with further discussion in meetings and in the new PR.

…script For a given commitment number, all revokeable outputs of HTLC transactions have the same script pubkey. If we know the secret for that commitment number, any outputs with that script pubkey are `RevokedOutput`s. This patch removes any checks on the input at the index of the revoked output, as these are not necessary.

tankyleo · 2025-01-27T19:54:41Z

@TheBlueMatt the very last commit here 22c9a5c implements a build_commitment_transaction call, let me know what you think.

a provided method - people who want custom stuff override, and people who want to stick to LN spec leave that alone.

move validate closing to main signer trait but the signature in a box that is generic this is the data the counterparty gave us to help us broadcast and mine that transaction

tankyleo mentioned this pull request Jan 8, 2025

Let ChannelSigner set to_remote, to_local, htlc tx scriptpubkeys #3454

Closed

tankyleo force-pushed the 2025-01-signer-spks branch from 508d577 to 5b57776 Compare January 10, 2025 22:54

tankyleo force-pushed the 2025-01-signer-spks branch 2 times, most recently from 1547436 to 72b317e Compare January 14, 2025 18:48

tankyleo added 14 commits January 14, 2025 23:57

Add ChannelSigner::get_channel_parameters

a775008

Ask ChannelSigner the weight of the punishment witness

70b9df6

for the revokeable scripts in the `to_local` and the htlc tx outputs.

Remove unused RevokedOutput fields

907684b

Remove unused RevokedHTLCOutput fields

b0a20d8

Ask ChannelSigner the weight of the htlc punishment witness

2868379

Let ChannelSigner build the counterparty htlc sweep witness

77d5e24

Remove unused CounterpartyOfferedHTLCOutput fields

563efd5

Remove unused CounterpartyReceivedHTLCOutput fields

b7a37ca

Ask ChannelSigner the weight of counterparty htlc output sweeps

35901b5

tankyleo force-pushed the 2025-01-signer-spks branch 2 times, most recently from 8000729 to 8a20905 Compare January 16, 2025 17:59

tankyleo added 9 commits January 20, 2025 17:29

Let sign_holder_commitment build the commitment tx witness

b88a2ac

Let unsafe_sign_holder_commitment build the commitment tx witness

4de22fa

Let sign_holder_htlc_transaction build the htlc tx witness

4c96432

Relax EcdsaChannelSigner to ChannelSigner in mod chain

9687da3

Ask ChannelSigner the weight of the htlc tx input

0e3227e

Let ChannelSigner set commit tx htlc output script pubkey

a9c3844

Remove TxCreationKeys from `CommitmentTransaction::internal_build_o…

ec40758

…utputs`

Let ChannelSigner set commit tx anchor output script pubkey

207b70f

Let ChannelSigner build the anchor sweep witness

557a754

tankyleo added 7 commits January 20, 2025 17:29

Delete StaticPaymentOutputDescriptor::witness_script

61fc584

Delete HTLCDescriptor::tx_input_witness

2ed352c

Delete TrustedCommitmentTransaction::get_htlc_sigs

fd1a47a

Move commitment tx sig validation from FundedChannel to `ChannelSig…

6ea5589

…ner`

Move htlc tx sig validation from FundedChannel to ChannelSigner

e2f73a5

Delete `InitialRemoteCommitmentReceiver::check_counterparty_commitmen…

ccf9824

…t_signature`

Call validate_counterparty_revocation on ChannelSigner directly

5fc079b

tankyleo force-pushed the 2025-01-signer-spks branch from 8a20905 to 5fc079b Compare January 20, 2025 17:48

TheBlueMatt reviewed Jan 20, 2025

View reviewed changes

tankyleo added 7 commits January 27, 2025 19:04

f: Make Counterparty{*}HTLCOutput ser backwards compatible

0c2b44b

f: Use ChannelSigner::get_anchor_txout to id the anchor output

7484c3b

Remove generic from CommitmentTransaction::internal_build_outputs

9ab6948

Let ChannelSigner build the outputs of the commit tx

bb16170

Let ChannelSigner build the inputs of the commit tx

09cbb8b

Let ChannelSigner build the commit tx

22c9a5c

tankyleo added 7 commits February 13, 2025 19:51

Add ChannelSigner::validate_closing_signature

9d05b78

move validate closing to main signer trait but the signature in a box that is generic this is the data the counterparty gave us to help us broadcast and mine that transaction

Add ChannelSigner::build_signed_closing_transaction

b06273b

Add ChannelSigner::provide_counterparty_parameters

e6e4acf

Add ChannelSigner::provide_funding_outpoint

70788e0

Provide counterparty parameters in accept channel checks

77b6fd4

Add ChannelSigner::get_funding_spk

e711b26

Add ChannelContext::get_funding_spk

bb13acb

tankyleo closed this Feb 14, 2025

tankyleo reopened this Feb 18, 2025

tankyleo force-pushed the 2025-01-signer-spks branch 2 times, most recently from 431f467 to bb13acb Compare February 18, 2025 05:54

tankyleo closed this Feb 18, 2025

tankyleo mentioned this pull request Feb 18, 2025

Custom Transactions Tracking Issue #3603

Open

24 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Let `ChannelSigner` set and spend LN scriptpubkeys #3512

Let `ChannelSigner` set and spend LN scriptpubkeys #3512

Uh oh!

tankyleo commented Jan 8, 2025 •

edited

Loading

Uh oh!

codecov bot commented Jan 13, 2025 •

edited

Loading

Uh oh!

TheBlueMatt Jan 20, 2025

Uh oh!

tankyleo Jan 21, 2025

Uh oh!

tankyleo Jan 22, 2025

Uh oh!

tankyleo Jan 22, 2025 •

edited

Loading

Uh oh!

tankyleo Jan 23, 2025

Uh oh!

TheBlueMatt Feb 21, 2025

Uh oh!

tankyleo commented Jan 27, 2025 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Let ChannelSigner set and spend LN scriptpubkeys #3512

Let ChannelSigner set and spend LN scriptpubkeys #3512

Uh oh!

Conversation

tankyleo commented Jan 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Jan 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

TheBlueMatt Jan 20, 2025

Choose a reason for hiding this comment

Uh oh!

tankyleo Jan 21, 2025

Choose a reason for hiding this comment

Uh oh!

tankyleo Jan 22, 2025

Choose a reason for hiding this comment

Uh oh!

tankyleo Jan 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tankyleo Jan 23, 2025

Choose a reason for hiding this comment

Uh oh!

TheBlueMatt Feb 21, 2025

Choose a reason for hiding this comment

Uh oh!

tankyleo commented Jan 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Let `ChannelSigner` set and spend LN scriptpubkeys #3512

Let `ChannelSigner` set and spend LN scriptpubkeys #3512

tankyleo commented Jan 8, 2025 •

edited

Loading

codecov bot commented Jan 13, 2025 •

edited

Loading

tankyleo Jan 22, 2025 •

edited

Loading

tankyleo commented Jan 27, 2025 •

edited

Loading